Skip to content

Conversation

@alwx
Copy link
Contributor

@alwx alwx commented Sep 19, 2025

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

After we've got rid of react-test-renderer, nothing was blocking us from upgrading core to 0.80. Because some mocks were missing in 0.80.0 (facebook/react-native#51993), the update was done to 0.80.1 straight away.
In addition to that, tests were updated (just because some of them required certain modifications after the RN version bump).

📝 Checklist

  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

#skip-changelog

@alwx alwx self-assigned this Sep 19, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Sep 19, 2025

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 484.43 ms 606.53 ms 122.10 ms
Size 43.75 MiB 47.99 MiB 4.23 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d1fd647+dirty 413.02 ms 459.72 ms 46.70 ms
170d5ea+dirty 407.92 ms 422.49 ms 14.57 ms
6479fd5+dirty 412.95 ms 434.02 ms 21.07 ms
1226664+dirty 347.45 ms 386.60 ms 39.15 ms
4604da9+dirty 366.44 ms 398.10 ms 31.66 ms
5ee3314+dirty 415.80 ms 426.14 ms 10.34 ms
b3b5b0d 399.82 ms 419.20 ms 19.38 ms
e2fa43d 451.68 ms 462.42 ms 10.74 ms
20d5eaa 377.62 ms 406.50 ms 28.88 ms
2adbd1e+dirty 433.98 ms 427.96 ms -6.02 ms

App size

Revision Plain With Sentry Diff
d1fd647+dirty 17.75 MiB 19.70 MiB 1.95 MiB
170d5ea+dirty 17.75 MiB 19.70 MiB 1.95 MiB
6479fd5+dirty 17.75 MiB 19.68 MiB 1.94 MiB
1226664+dirty 17.75 MiB 19.74 MiB 1.99 MiB
4604da9+dirty 17.75 MiB 19.74 MiB 2.00 MiB
5ee3314+dirty 17.75 MiB 19.70 MiB 1.95 MiB
b3b5b0d 17.75 MiB 19.68 MiB 1.94 MiB
e2fa43d 17.75 MiB 20.15 MiB 2.41 MiB
20d5eaa 17.75 MiB 20.15 MiB 2.41 MiB
2adbd1e+dirty 17.75 MiB 19.70 MiB 1.96 MiB

Previous results on branch: alwx/improvement/core-0.80

Startup times

Revision Plain With Sentry Diff
d2f50db+dirty 565.81 ms 578.64 ms 12.83 ms

App size

Revision Plain With Sentry Diff
d2f50db+dirty 17.75 MiB 19.68 MiB 1.94 MiB

@github-actions
Copy link
Contributor

github-actions bot commented Sep 19, 2025

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1214.89 ms 1214.81 ms -0.08 ms
Size 3.41 MiB 4.57 MiB 1.16 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d751a5d+dirty 1215.57 ms 1220.56 ms 4.99 ms
c08359e+dirty 1235.25 ms 1233.96 ms -1.29 ms
49ef936+dirty 1228.42 ms 1217.09 ms -11.33 ms
6a70a7e+dirty 1225.82 ms 1230.79 ms 4.98 ms
69602ce+dirty 1235.65 ms 1230.82 ms -4.83 ms
1e7a472+dirty 1223.39 ms 1232.12 ms 8.73 ms
5c16cdc+dirty 1209.32 ms 1210.67 ms 1.35 ms
7be1f99+dirty 1226.69 ms 1217.76 ms -8.93 ms
4604da9+dirty 1232.59 ms 1232.26 ms -0.33 ms
07808fb+dirty 1233.31 ms 1232.77 ms -0.54 ms

App size

Revision Plain With Sentry Diff
d751a5d+dirty 2.63 MiB 3.98 MiB 1.34 MiB
c08359e+dirty 2.63 MiB 3.81 MiB 1.18 MiB
49ef936+dirty 2.63 MiB 3.98 MiB 1.34 MiB
6a70a7e+dirty 2.63 MiB 3.98 MiB 1.34 MiB
69602ce+dirty 2.63 MiB 3.91 MiB 1.28 MiB
1e7a472+dirty 2.63 MiB 4.00 MiB 1.36 MiB
5c16cdc+dirty 2.63 MiB 3.96 MiB 1.33 MiB
7be1f99+dirty 2.63 MiB 3.81 MiB 1.18 MiB
4604da9+dirty 2.63 MiB 4.01 MiB 1.38 MiB
07808fb+dirty 2.63 MiB 3.99 MiB 1.36 MiB

Previous results on branch: alwx/improvement/core-0.80

Startup times

Revision Plain With Sentry Diff
d2f50db+dirty 1228.27 ms 1242.14 ms 13.87 ms

App size

Revision Plain With Sentry Diff
d2f50db+dirty 2.63 MiB 3.98 MiB 1.34 MiB

@github-actions
Copy link
Contributor

github-actions bot commented Sep 22, 2025

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 404.10 ms 458.28 ms 54.18 ms
Size 43.94 MiB 48.81 MiB 4.88 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
955f2eb+dirty 388.13 ms 433.56 ms 45.44 ms
2104bb9+dirty 313.00 ms 309.76 ms -3.24 ms
69602ce+dirty 375.37 ms 405.28 ms 29.91 ms
11ded16+dirty 309.23 ms 310.55 ms 1.33 ms
7be1f99+dirty 369.02 ms 399.60 ms 30.58 ms
6fee48d+dirty 370.23 ms 427.86 ms 57.63 ms
3bd3f0d+dirty 334.38 ms 402.19 ms 67.81 ms
5526494+dirty 380.79 ms 432.70 ms 51.91 ms
c94a927+dirty 411.32 ms 443.18 ms 31.86 ms
9f211e3+dirty 371.00 ms 432.51 ms 61.51 ms

App size

Revision Plain With Sentry Diff
955f2eb+dirty 7.15 MiB 8.42 MiB 1.27 MiB
2104bb9+dirty 7.15 MiB 8.46 MiB 1.30 MiB
69602ce+dirty 7.15 MiB 8.41 MiB 1.26 MiB
11ded16+dirty 7.15 MiB 8.46 MiB 1.31 MiB
7be1f99+dirty 7.15 MiB 8.42 MiB 1.27 MiB
6fee48d+dirty 7.15 MiB 8.41 MiB 1.26 MiB
3bd3f0d+dirty 7.15 MiB 8.43 MiB 1.28 MiB
5526494+dirty 7.15 MiB 8.41 MiB 1.26 MiB
c94a927+dirty 7.15 MiB 8.43 MiB 1.28 MiB
9f211e3+dirty 7.15 MiB 8.41 MiB 1.26 MiB

Previous results on branch: alwx/improvement/core-0.80

Startup times

Revision Plain With Sentry Diff
d2f50db+dirty 334.22 ms 358.23 ms 24.00 ms

App size

Revision Plain With Sentry Diff
d2f50db+dirty 7.15 MiB 8.41 MiB 1.26 MiB

@alwx alwx changed the title Bump core to RN 0.80 Bump core to RN 0.80.1 Sep 25, 2025
@alwx alwx marked this pull request as ready for review September 25, 2025 11:42
@antonis
Copy link
Contributor

antonis commented Sep 29, 2025

Note: I've took the liberty of adding the #skip-changelog to make the CI happy. Checking the latest bumps (#4560, #4331, #4173) since we usually don't communicate this kind of internal change.

],
"peerDependencies": {
"expo": ">=49.0.0",
"react": ">=17.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Why do we remove the "react" peer depencency?

includeUpdates: true,
}),
expect.anything(),
expect.toBeNil()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Do we know why the second parameter changed? Does it affect the profiler?

Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for you work on this @alwx 🙇
Overall the changes LGTM 🚀

I think the only blocking thing is the Native test failure on iOS. I haven't looked at it in detail but some of the errors might be related to the folly. We added some check here some time ago. Probably the Cocoa Tester needs something similar 🤔

@alwx alwx force-pushed the alwx/improvement/core-0.80 branch 2 times, most recently from 33bf13b to dd7d431 Compare September 30, 2025 08:59
@github-actions
Copy link
Contributor

github-actions bot commented Sep 30, 2025

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1224.29 ms 1224.90 ms 0.62 ms
Size 3.41 MiB 4.57 MiB 1.16 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d751a5d+dirty 1212.22 ms 1217.94 ms 5.71 ms
c08359e+dirty 1200.59 ms 1211.81 ms 11.22 ms
49ef936+dirty 1221.27 ms 1221.60 ms 0.34 ms
6a70a7e+dirty 1231.40 ms 1239.49 ms 8.09 ms
69602ce+dirty 1230.59 ms 1230.84 ms 0.24 ms
1e7a472+dirty 1237.44 ms 1231.14 ms -6.29 ms
5c16cdc+dirty 1235.67 ms 1241.18 ms 5.51 ms
7be1f99+dirty 1222.43 ms 1217.15 ms -5.28 ms
4604da9+dirty 1208.67 ms 1208.12 ms -0.54 ms
07808fb+dirty 1240.76 ms 1251.00 ms 10.24 ms

App size

Revision Plain With Sentry Diff
d751a5d+dirty 3.19 MiB 4.54 MiB 1.36 MiB
c08359e+dirty 3.19 MiB 4.38 MiB 1.19 MiB
49ef936+dirty 3.19 MiB 4.54 MiB 1.36 MiB
6a70a7e+dirty 3.19 MiB 4.54 MiB 1.36 MiB
69602ce+dirty 3.19 MiB 4.48 MiB 1.29 MiB
1e7a472+dirty 3.19 MiB 4.56 MiB 1.38 MiB
5c16cdc+dirty 3.19 MiB 4.53 MiB 1.34 MiB
7be1f99+dirty 3.19 MiB 4.38 MiB 1.19 MiB
4604da9+dirty 3.19 MiB 4.58 MiB 1.39 MiB
07808fb+dirty 3.19 MiB 4.56 MiB 1.37 MiB

Previous results on branch: alwx/improvement/core-0.80

Startup times

Revision Plain With Sentry Diff
d2f50db+dirty 1236.81 ms 1235.50 ms -1.31 ms

App size

Revision Plain With Sentry Diff
d2f50db+dirty 3.19 MiB 4.54 MiB 1.36 MiB

@alwx alwx force-pushed the alwx/improvement/core-0.80 branch 2 times, most recently from f293655 to 5f9c0aa Compare October 6, 2025 13:53
@lucas-zimerman
Copy link
Collaborator

@sentry review

Comment on lines 13 to +33
};
const mockedAppState: AppState & MockAppState = {
removeSubscription: jest.fn(),
listener: jest.fn(),
isAvailable: true,
currentState: 'active',
addEventListener: (_, listener) => {
mockedAppState.listener = listener;
return {
remove: mockedAppState.removeSubscription,
};
},
setState: (state: AppStateStatus) => {
mockedAppState.currentState = state;
mockedAppState.listener(state);
},
};
jest.mock('react-native/Libraries/AppState/AppState', () => mockedAppState);
jest.mock('react-native', () => {
const mockedAppState: AppState & MockAppState = {
removeSubscription: jest.fn(),
listener: jest.fn(),
isAvailable: true,
currentState: 'active',
addEventListener: jest.fn(),
setState: (state: AppStateStatus) => {
mockedAppState.currentState = state;
mockedAppState.listener(state);
},
};
return {
AppState: mockedAppState,
Platform: { OS: 'ios' },
NativeModules: {
RNSentry: {},
},
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mock setup creates a circular reference where mockedAppState.addEventListener and mockedAppState.setState reference mockedAppState before it's fully initialized. While this works in JavaScript due to hoisting, it's a code smell. Consider initializing addEventListener and setState after the object is created, or restructuring the mock to avoid this pattern.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +30 to +49
jest.mock('react-native', () => {
const mockedAppState: AppState & MockAppState = {
removeSubscription: jest.fn(),
listener: jest.fn(),
isAvailable: true,
currentState: 'active',
addEventListener: jest.fn(),
setState: (state: AppStateStatus) => {
mockedAppState.currentState = state;
mockedAppState.listener(state);
},
};
return {
AppState: mockedAppState,
Platform: { OS: 'ios' },
NativeModules: {
RNSentry: {},
},
};
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the idleNavigationSpan test, the mock setup creates a circular reference in the object initialization. Consider restructuring the mock to avoid this pattern for better code clarity and maintainability.

Did we get this right? 👍 / 👎 to inform future reviews.

showFeedbackButton();

fireEvent.press(getByText('Report a Bug'));
await waitFor(() => fireEvent.press(getByText('Report a Bug')));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On line 88, the pattern await waitFor(() => fireEvent.press(...)) is unusual. Typically, fireEvent.press() is synchronous and doesn't need to be wrapped in waitFor().
The waitFor() is usually used for waiting for asynchronous side effects after the event. Consider whether this should be fireEvent.press(...) followed by await waitFor(() => expect(...)) for better clarity.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +109 to +112
await waitFor(() => {
fireEvent.press(getByText('Report a Bug'));
fireEvent.press(getByText('Take a screenshot'));
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple instances of wrapping synchronous fireEvent.press() calls inside waitFor() (lines 109-112, 135-138, 167-170, 213-216) suggest a misunderstanding of the testing pattern. fireEvent operations are synchronous; only the assertions of their effects typically need waitFor(). This could lead to unnecessary delays in test execution.

Did we get this right? 👍 / 👎 to inform future reviews.

includeUpdates: true,
}),
expect.anything(),
expect.toBeNil()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from expect.anything() to expect.toBeNil() appears to be checking for a nil/null value. However, toBeNil() is not a standard Jest matcher. This might be a custom matcher from jest-extended, but it should be verified that this change correctly validates the expected behavior. Standard Jest would use expect(value).toBeNull() or expect(value).toBeUndefined().

Did we get this right? 👍 / 👎 to inform future reviews.

updateProps: {},
}),
expect.anything(),
expect.toBeNil(),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the previous comment, verify that expect.toBeNil() is the correct matcher and is properly imported/configured. If this is from jest-extended, ensure it's properly set up in the test configuration.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines 72 to 73
const versionNumber = parseFloat(rnVersion.replace(/[^\d.]/g, ''));
const shouldRunCodegen = platform === 'android' && versionNumber >= 0.80;
const shouldRunCodegen = platform === 'android' && versionNumber >= 0.69;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version threshold change from 0.80 to 0.69 for running codegen needs to be carefully verified. According to the repository context, Metro version pinning is applied only for React Native versions 0.80 and above. This change to 0.69 might require corresponding updates to the Metro version pinning logic in rn.patch.package.json.js to ensure consistency. Verify that RN 0.69 doesn't have the same Metro issues mentioned in the context.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines 107 to 115
const versionNumber = parseFloat(RNVersion.replace(/[^\d.]/g, ''));
if (versionNumber >= 0.80) {
if (versionNumber >= 0.69) {
execSync(`${patchScriptsDir}/rn.patch.package.json.js --path package.json`, {
stdio: 'inherit',
cwd: appDir,
env: env,
});
} else {
console.log(`Skipping rn.patch.package.json.js for RN ${RNVersion} (< 0.80)`);
console.log(`Skipping rn.patch.package.json.js for RN ${RNVersion} (< 0.69)`);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version threshold for applying rn.patch.package.json.js has been lowered from 0.80 to 0.69. According to the repository-specific context, Metro version pinning via this patch script is critical for RN 0.80+ to prevent E2E test failures. Ensure that applying this patch to RN 0.69 is safe and doesn't introduce unintended side effects, or update the patch script logic to handle different RN versions appropriately.

Did we get this right? 👍 / 👎 to inform future reviews.

"prettier": "^2.0.5",
"react": "18.3.1",
"react-native": "0.77.1",
"react": "19.1.0",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The update from React 18.3.1 to 19.1.0 is a major version upgrade that introduces breaking changes. Ensure that all React 19 breaking changes have been reviewed and that the codebase is compatible. Specifically, React 19 has changes to refs, context, and hooks behavior that may require code updates beyond just dependency version bumps.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines 652 to 653
animationType="none"
hardwareAccelerated={false}
onRequestClose={[Function]}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of the hardwareAccelerated={false} prop from Modal snapshots indicates a change in React Native's Modal component API between versions. While snapshot updates are expected with dependency upgrades, verify that this prop removal doesn't affect the actual runtime behavior of modals in the application, especially on Android where hardware acceleration can impact performance.

Did we get this right? 👍 / 👎 to inform future reviews.

@alwx alwx force-pushed the alwx/improvement/core-0.80 branch 4 times, most recently from f5710f4 to c695a04 Compare October 16, 2025 11:59
@alwx alwx force-pushed the alwx/improvement/core-0.80 branch 4 times, most recently from 73bb9e1 to 4a57918 Compare October 20, 2025 12:56
Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes LGTM 🚀
Thank you for all your work on this @alwx 🙇

@alwx alwx force-pushed the alwx/improvement/core-0.80 branch 4 times, most recently from f8c3f6b to 32d6cd0 Compare October 22, 2025 12:11
@lucas-zimerman
Copy link
Collaborator

Looking good! the changed kotlin files from the test app should be linted, after that we could merge this PR!

@alwx alwx force-pushed the alwx/improvement/core-0.80 branch 2 times, most recently from d553558 to df1c5b0 Compare October 28, 2025 09:47
@alwx alwx force-pushed the alwx/improvement/core-0.80 branch from 4917b07 to db5bea1 Compare October 29, 2025 13:09
@alwx alwx merged commit 8610fc8 into main Oct 31, 2025
87 of 92 checks passed
@alwx alwx deleted the alwx/improvement/core-0.80 branch October 31, 2025 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants